Skip to content

refactor(mocks): clean up Mock API surface#5314

Merged
thomhurst merged 1 commit intomainfrom
refactor/mock-api-surface-cleanup
Mar 30, 2026
Merged

refactor(mocks): clean up Mock API surface#5314
thomhurst merged 1 commit intomainfrom
refactor/mock-api-surface-cleanup

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • Move 6 framework-internal methods (RegisterFactory, RegisterMultiFactory, RegisterDelegateFactory, RegisterWrapFactory, TryCreateAutoMock, GetEngine) from Mock to a new [EditorBrowsable(Never)] MockRegistry class — completely removes them from Mock. autocomplete even in project-reference scenarios
  • Move 11 static helper methods that take Mock<T> as first parameter to instance members on Mock<T> (e.g. Mock.GetInvocations(mock)mock.Invocations, Mock.Reset(mock)mock.Reset())
  • Mock. autocomplete now shows only 5 entries: Of, OfDelegate, Wrap, Get, VerifyInOrder

Before → After

Before (static) After (instance)
Mock.GetInvocations(mock) mock.Invocations
Mock.GetBehavior(mock) mock.Behavior
Mock.Get/SetDefaultValueProvider(mock, ...) mock.DefaultValueProvider
Mock.SetupAllProperties(mock) mock.SetupAllProperties()
Mock.Reset(mock) mock.Reset()
Mock.VerifyAll(mock) mock.VerifyAll()
Mock.VerifyNoOtherCalls(mock) mock.VerifyNoOtherCalls()
Mock.GetDiagnostics(mock) mock.GetDiagnostics()
Mock.SetState(mock, s) mock.SetState(s)
Mock.InState(mock, s, a) mock.InState(s, a)

Test plan

  • All 679 TUnit.Mocks.Tests pass
  • All 22 source generator snapshot tests pass (snapshots updated)
  • All Roslyn variant generators build successfully
  • Documentation updated (advanced.md, verification.md, setup.md)

… to MockRegistry and instance helpers to Mock<T>

Move 6 framework-internal methods (RegisterFactory, RegisterMultiFactory, RegisterDelegateFactory,
RegisterWrapFactory, TryCreateAutoMock, GetEngine) to a new [EditorBrowsable(Never)] MockRegistry class.
Move 11 static helpers that take Mock<T> as first parameter to instance members on Mock<T>
(Invocations, Behavior, DefaultValueProvider, SetupAllProperties, Reset, VerifyAll,
VerifyNoOtherCalls, GetDiagnostics, SetState, InState).

Mock. autocomplete now shows only 5 entries: Of, OfDelegate, Wrap, Get, VerifyInOrder.
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Mock API Surface Refactor

This is a well-executed refactor that meaningfully improves the ergonomics of the mocking API. The changes are consistently applied across the source generator, tests, and documentation.

What works well

MockRegistry extraction — Moving the 6 framework-internal methods into a dedicated [EditorBrowsable(Never)] class is the right call. The previous approach of hiding generated-code-only members on the user-facing Mock static class was a leaky abstraction. Now Mock. autocomplete genuinely reflects the user-level surface, and MockRegistry acts as a clean internal seam between the generator and the runtime.

Promoting static helpers to instance membersmock.Reset(), mock.Invocations, mock.VerifyAll(), etc. are dramatically more idiomatic. The old Mock.Reset(mock) pattern was essentially simulating instance methods with a static helper, which IDE tooling doesn't optimally surface. The new API is what users will naturally reach for.

IMock delegation kept intact — The explicit interface implementations (void IMock.VerifyAll() => VerifyAll()) correctly preserve any code that goes through the IMock abstraction (e.g. MockRepository), with no behaviour change.

Source generator consistency — Every snapshot file is consistently updated to MockRegistry.RegisterFactory/MockRegistry.GetEngine. No half-migrated call sites remain.

InState thread safety — Saving/restoring _engine.PendingRequiredState in a try/finally is the right pattern for the ambient-context style of state scoping.

Observations and minor suggestions

MockRegistry is public but fully [EditorBrowsable(Never)] — This is the conventional pattern for generated-code entrypoints, but the class itself is still fully public, meaning it shows up in docs generators (DocFX, etc.) and is technically part of the public API contract for binary compatibility. If the intention is truly "internal to the generator", internal visibility would be a stronger guarantee. Since generated code lives in the same assembly it compiles into, internal would work fine and removes the need for the attribute entirely. If public is intentional (e.g. to support external generator extensions), the current approach is correct — just noting the trade-off.

GetEngine remains public on MockRegistryMockRegistry.GetEngine<T>(Mock<T>) is marked [EditorBrowsable(Never)] but is public, because the source-generated extension methods call it. This is the same situation as above. The 4 internal TryGet* helpers that were extracted back to MockRegistry are correctly internal, which is good.

Behavior as a read-only property vs MockBehaviormock.Behavior (new) replaces Mock.GetBehavior(mock) (old). There's no SetBehavior equivalent, which is intentional given behavior is set at construction time. The asymmetry is fine, but worth a quick doc comment note that behavior is construction-time-only (the current summary just says "Returns the mock behavior").

DefaultValueProvider as a read-write property — This is a clean improvement over the old separate GetDefaultValueProvider/SetDefaultValueProvider pair. No issues here.

InState captures this via the Action<Mock<T>> configure delegate — This is correct and necessary. One thing worth considering: if configure throws, PendingRequiredState is correctly restored via finally, which is good. The behaviour is consistent with the existing static version.

Breaking change — Existing user code calling Mock.GetInvocations(mock), Mock.Reset(mock), etc. will fail to compile after upgrading. This is expected and clearly documented in the PR description table. If TUnit.Mocks has not reached a stable 1.0, this is fine. If it has, a migration note in the changelog and a brief deprecation period (keeping the old statics as [Obsolete] for one minor version) would soften the upgrade path for existing consumers.

No [Obsolete] forwarding stubs — Related to the point above: since the old statics are fully removed rather than [Obsolete]-bridged, any downstream code calling the old API will get a hard compile error with no migration hint. Given this appears to be an actively evolving library (not a stable 1.0 release), this is probably fine — but something to consider if the library is already published on NuGet.

Summary

Clean, well-scoped refactor. The separation of MockRegistry (generator-internal) from Mock (user-facing) is architecturally sound, and moving helper methods to instance members on Mock<T> is the natural evolution of the API. All snapshot tests, source generator builders, test files, and documentation are consistently updated. The main consideration is whether to provide a one-version [Obsolete] bridge for existing users before doing the hard removal.

@thomhurst thomhurst enabled auto-merge (squash) March 30, 2026 18:26
@thomhurst thomhurst merged commit b1dd2a6 into main Mar 30, 2026
15 checks passed
@thomhurst thomhurst deleted the refactor/mock-api-surface-cleanup branch March 30, 2026 19:04
This was referenced Mar 30, 2026
intellitect-bot pushed a commit to IntelliTect/EssentialCSharp.Web that referenced this pull request Apr 1, 2026
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.23.7 to
1.24.13.

<details>
<summary>Release notes</summary>

_Sourced from [TUnit's
releases](https://github.com/thomhurst/TUnit/releases)._

## 1.24.13

<!-- Release notes generated using configuration in .github/release.yml
at v1.24.13 -->

## What's Changed
### Other Changes
* perf(mocks): optimize MockEngine for lower allocation and faster
verification by @​thomhurst in
thomhurst/TUnit#5319
* Remove defunct `UseTestingPlatformProtocol` reference for vscode by
@​erwinkramer in thomhurst/TUnit#5328
* perf(aspnetcore): prevent thread pool starvation during parallel
WebApplicationTest server init by @​thomhurst in
thomhurst/TUnit#5329
* fix TUnit0073 for when type from from another assembly by @​SimonCropp
in thomhurst/TUnit#5322
* Fix implicit conversion operators bypassed in property injection casts
by @​Copilot in thomhurst/TUnit#5317
* fix(mocks): skip non-virtual 'new' methods when discovering mockable
members by @​thomhurst in thomhurst/TUnit#5330
* feat(mocks): IFoo.Mock() discovery with generic fallback and ORP
resolution by @​thomhurst in
thomhurst/TUnit#5327
### Dependencies
* chore(deps): update tunit to 1.24.0 by @​thomhurst in
thomhurst/TUnit#5315
* chore(deps): update aspire to 13.2.1 by @​thomhurst in
thomhurst/TUnit#5323
* chore(deps): update verify to 31.14.0 by @​thomhurst in
thomhurst/TUnit#5325

## New Contributors
* @​erwinkramer made their first contribution in
thomhurst/TUnit#5328

**Full Changelog**:
thomhurst/TUnit@v1.24.0...v1.24.13

## 1.24.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.24.0 -->

## What's Changed
### Other Changes
* perf: optimize TUnit.Mocks hot paths by @​thomhurst in
thomhurst/TUnit#5304
* fix: resolve System.Memory version conflict on .NET Framework (net462)
by @​thomhurst in thomhurst/TUnit#5303
* fix: resolve CS0460/CS0122/CS0115 when mocking concrete classes from
external assemblies by @​thomhurst in
thomhurst/TUnit#5310
* feat(mocks): parameterless Returns() and ReturnsAsync() for async
methods by @​thomhurst in thomhurst/TUnit#5309
* Fix typo in NUnit manual migration guide by @​aa-ko in
thomhurst/TUnit#5312
* refactor(mocks): unify Mock.Of<T>() and Mock.OfPartial<T>() into
single API by @​thomhurst in
thomhurst/TUnit#5311
* refactor(mocks): clean up Mock API surface by @​thomhurst in
thomhurst/TUnit#5314
* refactor(mocks): remove generic/untyped overloads from public API by
@​thomhurst in thomhurst/TUnit#5313
### Dependencies
* chore(deps): update tunit to 1.23.7 by @​thomhurst in
thomhurst/TUnit#5305
* chore(deps): update dependency mockolate to 2.1.1 by @​thomhurst in
thomhurst/TUnit#5307

## New Contributors
* @​aa-ko made their first contribution in
thomhurst/TUnit#5312

**Full Changelog**:
thomhurst/TUnit@v1.23.7...v1.24.0

Commits viewable in [compare
view](thomhurst/TUnit@v1.23.7...v1.24.13).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=TUnit&package-manager=nuget&previous-version=1.23.7&new-version=1.24.13)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant